Repository navigation
Added basic support for redis sentinel based on https://github.com/Namoshek/laravel-redis-sentinel - #184
Open
Jacq wants to merge 15 commits into
Open
Added basic support for redis sentinel based on https://github.com/Namoshek/laravel-redis-sentinel#184Jacq wants to merge 15 commits into
Jacq wants to merge 15 commits into
Conversation
Member
|
Hey, thank you for your work. Would it be possible to add tests based on |
Author
Already added some test and performed a full refactor based on a different repo, using phpredis sentinel support. |
Jacq
force-pushed
the
main
branch
2 times, most recently
from
April 18, 2025 17:19
e1a542d to
e531f94
Compare
Author
|
Added tests, ready to merge. |
Brings in the 12 upstream commits since April 2025: the str_replace fix when reading metrics from Redis (PromPHP#185), Predis support (PromPHP#186) with the Redis storage split into AbstractRedis + RedisClients, PHP 8.2+ as minimum, PHP 8.5 in CI and guzzle 8 allowed. Upstream moved the connection handling from Storage\Redis to RedisClients\PHPRedis, so the Sentinel support is re-applied there: Redis keeps the sentinel options and its public API (updateSentinelPrimary, getRedisSentinel, fromExistingConnection with an optional \RedisSentinel), and PHPRedis::ensureOpenConnection() asks Sentinel for the primary before connecting. The reconnect loop is fixed on the way: - it now retries: it only caught \RedisException, but getMaster() and connectToServer() already wrap it in StorageException, so no retry ever happened; the retry decision now looks at the wrapped \RedisException; - 'reconnect' => -1 now retries indefinitely as documented; before, `while ($retries <= -1)` never ran and the client connected to the default host without asking Sentinel; - running out of retries throws the last error instead of carrying on with no connection. getRedisSentinel() throws StorageException when Sentinel is disabled instead of a fatal error on null. Tests: two new Sentinel tests (reconnect -1 gets the primary; exhausting retries throws), and the misplaced @PHPStan-Ignore and trailing whitespace in RedisSentinelTest that made phpstan/phpcs fail. Full suite on PHP 8.4 against Redis 7 + Sentinel: 4211 tests OK (138 APC/APCng skipped, no apcu), phpstan and phpcs clean. AI-Assisted-By: Claude Opus 5.5 (claude-opus-5-5, Anthropic) AI-Oversight: merge, conflict resolution, reconnect fixes and tests generated and run by the AI; the author reviews the diff and approves the commit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merged and refactor code with from old fork #https://github.com/ml65/prometheus_client_php that supports redis sentinel by updating master address/port options before redis connections.